Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(server) Fix empty response #15322

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

fix(server) Fix empty response #15322

wants to merge 6 commits into from
+45 −10

Conversation

cirospaciari
Copy link
Member

@cirospaciari cirospaciari commented Nov 22, 2024 •
edited
Loading

What does this PR do?

Fix: #15320

This fix responses when the stream is early closed without sending any data.

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

How did you verify your code works?

New tests + existing tests

@robobun
Copy link

robobun commented Nov 22, 2024 •
edited
Loading

❌ @cirospaciari, your commit 8fe3b2e has 20 failures in #7404:

  • test/js/web/timers/setInterval.test.js - 1 failing on 🍎 13 x64
  • test/js/web/fetch/body-stream.test.ts - timeout on 🍎 13 x64
  • test/js/bun/shell/leak.test.ts - timeout on 🍎 13 x64
  • test/js/bun/http/serve-body-leak.test.ts - timeout on 🍎 13 x64
  • test/js/web/timers/setTimeout.test.js - 1 failing on 🐧 22.04 x64
  • test/package.json - code 1 on 🍎 14 aarch64
  • package.json - code 1 on 🍎 14 aarch64
  • package.json - code 1 on 🍎 14 aarch64
  • test/js/bun/plugin/plugins.test.ts - segmentation fault on 🐧 24.04 aarch64
  • test/regression/issue/09041.test.ts - 1 failing on 🐧 3.20 x64
  • test/cli/hot/hot.test.ts - timeout on 🐧 3.20 x64
  • test/cli/hot/hot.test.ts - timeout on 🐧 24.04 aarch64
  • test/integration/next-pages/test/dev-server.test.ts - 1 failing on 🐧 3.20 x64
  • test/v8/v8.test.ts - 22 failing on 🐧 3.20 aarch64
  • test/v8/v8.test.ts - 22 failing on 🐧 3.20 x64
  • test/v8/v8.test.ts - 22 failing on 🐧 3.20 x64-baseline
  • test/js/third_party/grpc-js/test-pick-first.test.ts - 1 failing on 🐧 11 x64-baseline
  • test/js/bun/util/filesink.test.ts - 1 failing on 🐧 24.04 x64-baseline
  • test/js/bun/util/filesink.test.ts - 1 failing on 🐧 3.20 x64
  • test/js/third_party/pino/pino.test.js - segmentation fault at address 0x290e8a9b878 on 🪟 2019 x64-baseline
  • test/js/third_party/pino/pino.test.js - segmentation fault at address 0x283c5cc4a80 on 🪟 2022 x64
  • test/js/third_party/pino/pino.test.js - segmentation fault at address 0x197c8ee30a8 on 🪟 2025 x64-baseline
  • test/js/third_party/pino/pino.test.js - segmentation fault at address 0x28f7f3a6940 on 🪟 2019 x64
  • test/js/node/fs/fs.test.ts - 1 failing on 🪟 2022 x64
  • test/cli/install/registry/bun-install-registry.test.ts - 1 failing on 🪟 2025 x64-baseline
  • test/cli/install/registry/bun-install-registry.test.ts - 1 failing on 🐧 3.20 aarch64
  • test/cli/run/require-cache.test.ts - 1 failing on 🪟 2022 x64
  • test/cli/run/require-cache.test.ts - 1 failing on 🪟 2025 x64
  • test/cli/run/require-cache.test.ts - 1 failing on 🪟 2022 x64-baseline
  • test/integration/next-pages/test/next-build.test.ts - 1 failing on 🪟 2019 x64-baseline
  • test/js/node/dns/node-dns.test.js - 1 failing on 🪟 2019 x64-baseline
  • test/js/node/dns/node-dns.test.js - 1 failing on 🪟 2019 x64
    		•

    @cirospaciari cirospaciari force-pushed the ciro/fix-15320 branch 2 times, most recently from 5d0d6bb to 85f0e65 Compare November 22, 2024 23:29
    @cirospaciari cirospaciari changed the title fix empty stream responses fix(server/fetch) Fix empty response and make fetch more reliable Nov 24, 2024
    @cirospaciari cirospaciari marked this pull request as draft November 26, 2024 19:43
    @cirospaciari cirospaciari force-pushed the ciro/fix-15320 branch 2 times, most recently from e807fc0 to 4766923 Compare November 26, 2024 22:08
    @cirospaciari cirospaciari changed the title fix(server/fetch) Fix empty response and make fetch more reliable fix(server) Fix empty response Nov 26, 2024
    @cirospaciari cirospaciari marked this pull request as ready for review November 26, 2024 22:12
    @cirospaciari cirospaciari marked this pull request as draft November 27, 2024 00:57
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @Jarred-Sumner Jarred-Sumner Jarred-Sumner left review comments

    At least 0 approving reviews are required to merge this pull request.

    Assignees

    @Jarred-Sumner Jarred-Sumner

    Labels
    None yet
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    Bun serve with a Response with empty ReadableStream won't redirect.
    3 participants
    @cirospaciari @robobun @Jarred-Sumner